feat: add code-review-checklist skill for code review guidance - #1172

Thanks for the follow-up on #903 and for narrowing scope to a single skill — that's the right instinct. The problem is substance, not format: this checklist asserts specific facts about the codebase (e.g. the project-files → auth → logger → project-files import cycle, cli/src/utils/auth.ts, cli/src/state/freebuff-session-store.ts, cli/src/hooks/use-freebuff-session.ts, the none → active → ended → none session state machine) that read as plausible but unverified — this is a squashed public mirror, and there's no way to confirm these paths or invariants actually match the current private tree. A skill file that maintainers rely on for review guidance needs to be accurate, not just repo-shaped.

Separately, most of sections 1, 2, 4, and 5 (no any, validate input, don't log secrets, test both success/error paths) are generic software-engineering advice that isn't specific to freebuff and doesn't need to live in a project skill — it applies to any TypeScript codebase.

If you want to make this land, the path is: verify every concrete claim (file paths, the import cycle, the session state machine) against actual current source, cite them accurately, and drop the generic boilerplate so the skill is dense with things a generic checklist wouldn't already tell a reviewer. As it stands, this risks pointing reviewers at files or invariants that may not exist or may be stale, which is worse than no checklist at all.

Thanks for the direct feedback. Rewrote the skill from scratch based on your two main points:

1. Verified every claim against current source

• Removed the false import cycle claim — project-files.ts imports from ./utils/config-dir, not ./utils/auth. No cycle exists on current main.
• Fixed the session state machine — the actual FreebuffSessionServerResponse has 12+ statuses (none, active, ended, superseded, country_blocked, banned, rate_limited, spend_limited, ip_capped, model_locked, model_unavailable, premium_slot_taken, takeover_prompt), not the 3-state cycle I originally claimed.
• All file paths verified via glob and direct source reading.

2. Removed generic boilerplate

Dropped sections on "no any types", "validate input", "don't log secrets", "test both paths" — none of that is freebuff-specific.

What's left is dense, verified, and specific:

• Session lifecycle with all 12+ statuses and actual transition paths
• Auth flow step-by-step with file paths and line numbers
• Known pitfalls: bun mock leakage, IS_FREEBUFF guards, config directory handling
• Risk table mapping files to why they break things

Every claim now has a verified source path. No generic advice remains.